Skip to content

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Nov 26, 2024

Motivation:

A number of things have changed since the generated code was initially designed, which means it can be simplified and improved.

Modifications:

  • As services are namespaced within a namespace-service enum (rather than within a service enum in a namespace enum), they no longer need to be grouped by namespace.
  • As deployment targets must be set in the package manifest, the generated code no longer needs to be annotated with availability guards so these have been removed.

Result:

Simpler code generation, better generated code.

Motivation:

A number of things have changed since the generated code was initially
designed, which means it can be simplified and improved.

Modifications:

- As services are namespaced within a namespace-service enum (rather
  than within a service enum in a namespace enum), they no longer need
  to be grouped by namespace.
- As deployment targets must be set in the package manifest, the
  generated code no longer needs to be annotated with availability
  guards so these have been removed.

Result:

Simpler code generation, better generated code.
@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Nov 26, 2024
@glbrntt glbrntt enabled auto-merge (squash) November 26, 2024 13:19
if let comment = description.comment { renderComment(comment) }
let item = description.item
renderCodeBlockItem(item)
if let item = description.item {
Copy link
Collaborator

@rnro rnro Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous style had a cleaner call site; what was the impetus to change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

description.item is now Optional, so we need this change (or for renderCodeBlockItem to take an optional)

Copy link
Collaborator

@rnro rnro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question otherwise it looks good

@glbrntt glbrntt merged commit d70fbad into grpc:main Nov 26, 2024
44 of 46 checks passed
@glbrntt glbrntt deleted the v2/03-simplify-generated-code branch November 26, 2024 13:33
glbrntt pushed a commit that referenced this pull request Dec 4, 2024
## Motivation
#2131 removed all usages of
availability guards. However, the `guarded` Declaration case was not
removed even though it's unused.

## Modifications
This PR removes the `guarded` case since it's not used anywhere anymore.

## Result
Cleaner codebase.
@rnro rnro mentioned this pull request Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants